Summary

Surgical 3-hunk change adding allowed_non_write_users: "*" to the two jobs that take input from non-maintainers (pr-review via pull_request_target, issue-handler via issues / issue_comment), plus a 12-line header block documenting the residual prompt-injection risk and the escape hatch (swap * for a literal username list). The scoping is correct: pr-comment is already filtered to same-repo PRs (head.repo.full_name == github.repository, line 334) and release-notes runs only on maintainer-owned tag pushes via workflow_run — adding the bypass to either would be a no-op. The single most important thing for the author/reviewer to know: this does widen the attack surface in exchange for fork-PR ergonomics, and the mitigation story leans entirely on upstream (claude-code-action) defense-in-depth rather than anything in this repo.

Issues Found

🟡 Important — claude_args for pr-review / issue-handler includes Edit,Write against repo files (claude.yml:322, 438)

Both fork-reachable jobs ship the same toolset:

--allowedTools Edit,Read,Write,Grep,Glob,Bash

The prompts only ever ask Claude to write /tmp/review.md / /tmp/reply.md and post via gh, so Edit against repo files isn't load-bearing. With allowed_non_write_users: "*" now letting an external author trigger these, a successful prompt injection has Edit + Write + Bash against the checked-out base-branch tree. The header already names secret exfiltration as the headline risk; arbitrary file writes (e.g. dropping a payload into a workflow file the next maintainer-triggered run picks up) is a quieter variant worth closing.

This isn't blocking — the PR is correctly scoped and pull_request_target itself doesn't push the worktree anywhere — but if you want the bypass to be defensible long-term, the cheapest follow-up is to either:

• Drop Edit from these two jobs' claude_args (review/reply doesn't need it), or
• Add a --disallowedTools line restricting Bash patterns (git push, curl, wget, nc, etc.) for the fork-reachable surface.

Worth a follow-up issue rather than blocking this PR, since the trade-off is documented and reverting is one config flip.

🟢 Minor — Header reference to src/github/validation/permissions.ts is ambiguous (claude.yml:39)

// no `author_association` values supported — checked
// upstream `src/github/validation/permissions.ts`, only literal usernames or `*`

Reads as a path in this repo on first scan. A future maintainer trying to verify the claim will grep src/github/... and come up empty. Suggest naming the source repo + file explicitly:

# this to a literal username list (no `author_association` values supported — verified
# in upstream anthropics/claude-code-action `src/github/validation/permissions.ts`,
# which only accepts literal usernames or `*`).

🟢 Minor — Test plan correctly notes self-test isn't possible, but the verification log line is worth pinning

The PR description references the upstream warning string ⚠️ SECURITY WARNING: Bypassing write permission check for <user> due to allowed_non_write_users configuration. That string is upstream-controlled and could change between SHAs. Once verified post-merge, consider adding it to the file header as the canonical signal so future-us doesn't re-derive it. Optional, not blocking.

Strengths

• Header comment is genuinely useful, not just performative. Names the threat (prompt-injection in fork diff → Bash exfiltration via ANTHROPIC_API_KEY / GITHUB_TOKEN), the mitigations actually in play (CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1, pinned bun, hardened PATH), and the explicit escape hatch (literal username list). This is the rare "we accepted residual risk" comment that future-us can actually act on.
• Scoping is right. Touching only pr-review and issue-handler — and explicitly not pr-comment (head.repo.full_name == github.repository at line 334 makes the bypass moot) or release-notes (workflow_run from maintainer tag push, actor is always a write user) — shows the author traced each job's trigger surface rather than carpet-bombing the file.
• Test plan honestly acknowledges the pull_request_target self-test gap. The post-merge verification path (re-trigger PR feat(agents): split ChatAgent into Chat, FileIO, and DocumentQA agent… #924, confirm bypass log line) is concrete and falsifiable.

Verdict

Approve with suggestions — change is minimal, scoped correctly, and the security trade-off is documented in the right place (the workflow header, not a buried PR comment). The 🟡 on claude_args toolset is a hardening follow-up, not a blocker; the 🟢 items are textual. Safe to merge once the upstream-path clarification on line 39 is folded in.